Conversation
When a tuple with an unbounded entry (e.g., tuple[int, *tuple[str, ...], int]) is matched against a pattern shorter than the tuple, the unbounded entry is removed to make lengths match. However, this loses the information that the tuple was originally variadic and could have different lengths. The fix tracks when an unbounded entry is removed (removedIndeterminate flag) and marks such matches as isPotentialNoMatch=true. This prevents the entire variadic tuple from being eliminated in negative narrowing, preserving reachability of subsequent match cases.
This comment has been minimized.
This comment has been minimized.
|
|
||
| # Potential venv | ||
| .venv No newline at end of file | ||
| .venv |
There was a problem hiding this comment.
Copilot generated:
-134
Remove this entire hunk from the PR. These additions (serena/, .beads/, AGENTS.md) are unrelated to the pattern matching fix and appear to be local dev environment artifacts. Notably, AGENTS.md exists at the repo root and is already tracked — adding it to .gitignore signals wrong intent and could confuse contributors. Keep the PR focused on the bug fix.
|
|
||
| // If the tuple contains an indeterminate entry, expand or remove that | ||
| // entry to match the length of the pattern if possible. | ||
| let expandedIndeterminate = false; |
There was a problem hiding this comment.
Copilot generated:
-1430
The comment documents downstream entanglement mechanics ("tupleIndeterminateIndex becomes -1, making isUnboundedTuple false... isPotentialNoMatch=true prevents incorrect elimination..."). While accurate today, this creates a maintenance trap — if anyone reorders the downstream isDefiniteMatch / isUnboundedTuple checks, the safety argument becomes silently stale. Consider simplifying to state intent: "Track that the original tuple was variadic; this information is lost when the unbounded entry is spliced out, and is needed to prevent incorrect definite-match classification."
| @@ -1435,6 +1441,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
Copilot generated:
-1442
After tupleIndeterminateIndex = -1, the resulting SequencePatternInfo will have isUnboundedTuple: false even though the original tuple IS unbounded. This works today because isPotentialNoMatch = true prevents elimination before isUnboundedTuple is consulted, but any future consumer of isUnboundedTuple that trusts it as ground truth will get incorrect information. Consider: isUnboundedTuple: removedIndeterminate || tupleIndeterminateIndex >= 0 at the push site (around line 1510) to preserve semantic accuracy.
|
|
||
| // If the tuple contains an indeterminate entry, expand or remove that | ||
| // entry to match the length of the pattern if possible. | ||
| let expandedIndeterminate = false; |
There was a problem hiding this comment.
Copilot generated:
-1430
Rewrite this comment to state intent, not downstream mechanics (unresolved from prior review). The current text documents a fragile ordering dependency ("isPotentialNoMatch=true prevents incorrect elimination at the isDefiniteMatch check before the isUnboundedTuple guard matters"). If anyone reorders those downstream checks, this safety argument becomes silently stale. Suggested replacement:
// Track that the original tuple was variadic; this information is lost when
// the unbounded entry is spliced out, and is needed to prevent incorrect
// definite-match classification downstream.
let removedIndeterminate = false;
rchiodo
left a comment
rchiodo
left a comment
There was a problem hiding this comment.
Approved via Review Center.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Fix pattern matching regression for variadic tuple types
When a tuple with an unbounded entry (e.g., tuple[int, *tuple[str, ...], int])
is matched against a pattern shorter than the tuple, the unbounded entry is
removed to make lengths match. However, this loses the information that the
tuple was originally variadic and could have different lengths.
The fix tracks when an unbounded entry is removed (removedIndeterminate flag)
and marks such matches as isPotentialNoMatch=true. This prevents the entire
variadic tuple from being eliminated in negative narrowing, preserving
reachability of subsequent match cases.
Fixes #10845